Address open issues #1, #2, #3, #6, #7, #8, #9, #12, #13, #14#34
Merged
Conversation
Closes the security hygiene gaps for the bootstrap demo path and the external password-manager fallback provider. #14: Gate the plaintext demo credentials.json behind APW_DEMO=1 in both the Rust CLI (`native_app::ensure_default_credentials_file`) and the Swift broker (`BrokerCore.ensureCredentialsFile`). Without APW_DEMO, `apw login` returns a typed `no_credential_source` error for example.com instead of silently falling back to bundled placeholder credentials. The "demo credentials created" notice is now a warning and only fires on the demo path. Regression tests assert that a fresh runtime directory does not contain credentials.json after doctor / install / broker startup. #1: Validate the external fallback provider path before exec — reject ~-prefixed paths, canonicalize via `fs::canonicalize` (resolving symlinks), require a regular file owned by the current euid with the execute bit set and not world-writable. Failures surface as typed `InvalidConfig` errors. The validation is best-effort against TOCTOU; a fully race-free implementation would require fexecve which is not portable. #3: Bound external fallback provider exec — wrap each invocation in a wall-clock timeout (`APW_FALLBACK_TIMEOUT_MS`, default 15s) that kills the child via SIGKILL on overrun, and a per-process invocation cap (`APW_FALLBACK_INVOCATION_LIMIT`, default 5) that returns a typed error when exceeded. Both behaviors covered by serial integration tests using sleep-30 and empty-array provider stubs. Docs in SECURITY_POSTURE_AND_TESTING.md and INSTALLATION.md describe the new defaults, the env-var overrides, and the demo opt-in. https://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
#2: Promote the broker IPC timeout to a documented constant on both sides — `BROKER_REQUEST_TIMEOUT_MS` in rust/src/native_app.rs (3000 ms) and `brokerRequestTimeoutMs` in BrokerCore.swift, the latter applied to each accepted client via `SO_RCVTIMEO` / `SO_SNDTIMEO` so a hung peer cannot block the broker indefinitely. Adds a regression test that parks a Unix-socket acceptor that never replies and asserts the CLI returns within `BROKER_REQUEST_TIMEOUT_MS + 1s` instead of hanging. SECURITY_POSTURE_AND_TESTING.md gains a "Timeouts and failure modes" section listing the values and behavior. #9: Add a one-line stderr deprecation warning to every CLI subcommand that routes through the legacy daemon path (`start`, `pw`, `otp`, `auth`), suppressed in `--json` mode via the existing logging gate so machine-readable output stays clean. Each command's `--help` (`long_about`) is prefixed with a `DEPRECATED:` banner and points at docs/MIGRATION_AND_PARITY.md, which now lists v2.1.0 as the planned removal milestone with replacements. A regression test in rust/tests/legacy_parity.rs asserts the warning appears for `apw pw`, `apw otp`, and `apw auth`. https://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
#12: New `rust/src/doctor.rs` module that probes the local environment and yields a typed `DoctorCheck` per concern (xcodebuild, rust-toolchain, detect-secrets, Apple Developer ID Application identity, native-app bundle install state). Each check includes a status, a human message, an optional remediation hint, and an optional detected-version string. `apw doctor` now appends the env checks under `payload.environment` and, in non-JSON mode, prints `[OK]/[WARN]/[FAIL]: name: message` lines on stderr while leaving stdout-JSON parseable. New `--ci` flag emits the structured array on its own for CI consumers (always JSON). When CI=true, the doctor also probes RUNNER_LABELS to sanity-check runner pool selection. Probes are bounded by a 3 s timeout so a misconfigured shim cannot hang the doctor. `docs/bootstrap/onboarding.md` now references `apw doctor` as the first step for new contributors. #8: New `docs/DOMAIN_EXPANSION.md` is the operator playbook for extending the v2 native app beyond the bundled example.com demo: config field, entitlement update, AASA hosting requirements, rebuild / re-sign / re-notarize flow, and verification via `apw doctor`. Doctor gains an `associated-domains` check that probes each domain listed in `APW_AASA_DOMAINS` for a reachable AASA file at `/.well-known/apple-app-site-association`. The env-var gate lets this ship ahead of the `supportedDomains` config schema change without breaking existing installs. Failing probes return a typed `[FAIL]` with a remediation pointer to the playbook. https://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
#6: New `scripts/render-homebrew-formula.sh` reads the committed formula at `packaging/homebrew/apw.rb` as a template, validates the provided version (SemVer) and sha256 (64-char hex), and emits a fully rendered formula on stdout. The release workflow gains a `homebrew-tap` job that runs after the main release, downloads the freshly-published source tarball, computes its sha256, renders the formula, and opens a PR against `omt-global/homebrew-apw` when `HOMEBREW_TAP_TOKEN` is configured. The job is `continue-on-error` so a failed tap update never blocks a release. When the token is absent the job emits a `::warning::` pointing operators at the manual render command. INSTALLATION.md documents the helper. #7: New `Apple notarize APW.app` step in the release job signs the APW.app bundle with a Developer ID Application certificate (imported into a short-lived keychain to avoid login-keychain collisions on shared runners), submits the signed zip to Apple Notary Service via `xcrun notarytool submit --wait`, staples the resulting ticket, and re-zips the stapled bundle as `dist/APW-<tag>.zip`. The publish step uploads both the existing source tarball and the notarized zip; `fail_on_unmatched_files: false` keeps non-tag dispatch runs working when the notarized zip is absent. The notarize step skips cleanly on non-tag builds and on tag builds where any of the required Apple secrets are absent, emitting a `::warning::` instead of failing. All Apple secrets are exposed at the job level so the per-step `if:` can check `env.APPLE_*_PRIVATE_KEY != ''` without leaking values. docs/bootstrap/onboarding.md lists the full secret matrix. https://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
#13: New `AuthenticationServicesBroker.swift` introduces: - A `CredentialBroker` protocol so `BrokerCore` can dispatch login requests without hard-binding to AuthenticationServices. - An `AppleAuthenticationServicesBroker` implementation that builds an `ASAuthorizationPasswordRequest`, drives `ASAuthorizationController` on the main thread, bridges the async delegate callback back to the broker's worker thread via `DispatchSemaphore`, and bounds the wait by `brokerRequestTimeoutMs` so a hung credential picker cannot block the broker. - A stable `BrokerErrorCode` enum (`canceled` / `failed` / `invalidResponse` / `notHandled` / `unknown`, plus `unsupportedDomain` / `noCredentialSource`) and a `brokerStatus(for:)` mapper that aligns those codes with the Rust `Status` enum on the wire (1 / 104 / 3). - A `defaultCredentialBroker()` factory that returns the real broker on Apple targets and `nil` elsewhere. `BrokerCore.loginResponse` now routes through the injected broker when `APW_DEMO` is unset, mapping outcomes onto the existing wire envelope: success returns `transport: "authentication_services"` with `userMediated: true`; denial returns the typed "User denied" error; failures return the `BrokerErrorCode` plus the Rust-mapped integer status. When no broker is wired, the response remains `no_credential_source`. `BrokerCoreTests` adds a `StubCredentialBroker` and four new tests covering success, denied, canceled, and invalidResponse outcomes plus a table-driven assertion on `brokerStatus(for:)`. The default test server now injects `nil` for the broker so XCTest never triggers a real `ASAuthorizationController` request. NATIVE_ONLY_REDESIGN.md gains a Phase 3 status block listing what landed and the remaining exit blocker (notarized-build verification on a real macOS host with associated-domain entitlements). README.md now distinguishes the real `apw login <vault>` flow from the `APW_DEMO=1`-gated bootstrap. Caveat: this scaffold cannot be compiled or run from the CI Linux box. The Swift surface follows the public AS API but the end-to-end behavior (associated-domain matching, run-loop pumping inside an LSUIElement broker, ASAuthorizationError code stability) requires verification on macOS — flagged inline with `TODO(#13)`. https://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd6b52a1b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1, #2, #3, #6, #7, #8, #9, #12, #13, #14.
Summary
d957d84): gate democredentials.jsonbehindAPW_DEMO=1(Rust + Swift); harden external fallback provider — canonicalize, owner/perm/exec-bit checks, exec timeout (APW_FALLBACK_TIMEOUT_MS), per-process invocation cap (APW_FALLBACK_INVOCATION_LIMIT).6618ac3): sharedBROKER_REQUEST_TIMEOUT_MSconstant on both sides withSO_RCVTIMEO/SO_SNDTIMEOon the Swift broker; one-line stderr deprecation warning +--helpbanner on every legacy daemon CLI path.apw doctorshould surface missing or misconfigured self-hosted runner requirements #12, Feature: expand associated-domain support beyond the demo domain #8 (10e614e): newrust/src/doctor.rswith structuredDoctorCheckprobes (xcodebuild, rustc, detect-secrets, Developer ID Application identity, native-app bundle, runner labels, AASA reachability) +apw doctor --ci; newdocs/DOMAIN_EXPANSION.mdoperator playbook.9a01b99):scripts/render-homebrew-formula.sh+homebrew-tapworkflow job that PRs into the tap repo whenHOMEBREW_TAP_TOKENis set; Apple notarization step skeleton (signs, notarizes, staples, re-zips) gated on tag builds + Apple secrets.bd6b52a):AuthenticationServicesBroker.swiftscaffold withCredentialBrokerprotocol,AppleAuthenticationServicesBrokerdrivingASAuthorizationControlleron the main thread viaDispatchSemaphore, stableBrokerErrorCodemapping.BrokerCore.loginResponsenow routes through the broker outside demo mode.Each issue has a per-issue comment listing what landed, what's still open, and the relevant commit SHA.
Verified on this branch
cargo fmt -- --checkcleancargo clippy --all-targets -- -D warningscleancargo test --all-targets: 173 passing (143 lib + 6 + 8 + 6 + 10 across integration suites)python3 -c "yaml.safe_load(...)"onrelease.ymlNot verifiable from this branch
xcodebuildon the Linux runner). Marked inline withTODO(#13)for follow-up validation on a notarized macOS host with associated-domain entitlements wired.APPLE_*secrets configured on the self-hosted macOS runner).omt-global/homebrew-apwrepo +HOMEBREW_TAP_TOKEN).Test plan
cargo test --all-targetson a fresh checkoutswift testfromnative-app/to exercise the newBrokerCoreTestsbroker outcome casesapw doctoroutput (human +--ciJSON) on a developer macOS boxAPW_DEMO=1 apw login https://example.comreturns the demo credential;apw loginwithoutAPW_DEMOreturnsno_credential_source~/-prefixed path → expect typedInvalidConfigerrorAPPLE_*secrets configured to validate the notarization stephttps://claude.ai/code/session_015zj6QWfz9zWVEe16GDRk5u
Generated by Claude Code